Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: improve user experience in forms #801

Merged
merged 4 commits into from
Aug 29, 2023
Merged

feat: improve user experience in forms #801

merged 4 commits into from
Aug 29, 2023

Conversation

sammyskills
Copy link
Contributor

Currently, when filling forms, there is no indication of the field being filled (or what is expected of the user) after a user has entered value in the input fields. If a user forgets what was in the placeholder before filling the form, s/he will need to delete the entire entry in the input to know what the field expects. See screenshot below:

image

This PR improves user experience by providing a floating label to indicate the field that is being filled. See screenshot below:

image

@sammyskills
Copy link
Contributor Author

PHPUnit Tests NOT associated with this PR failed:

1) Tests\Authentication\Filters\SessionFilterTest::testBlocksInactiveUsers
Test code or tested code did not (only) close its own output buffers

phpvfscomposer:///home/runner/work/shield/shield/vendor/phpunit/phpunit/phpunit:106

2) Tests\Authentication\Filters\SessionFilterTest::testStoreRedirectsToEntraceUrlIntoSession
Test code or tested code did not (only) close its own output buffers

phpvfscomposer:///home/runner/work/shield/shield/vendor/phpunit/phpunit/phpunit:106

3) Tests\Authentication\Filters\SessionFilterTest::testFilterNotAuthorized
Test code or tested code did not (only) close its own output buffers

phpvfscomposer:///home/runner/work/shield/shield/vendor/phpunit/phpunit/phpunit:106

4) Tests\Authentication\Filters\ChainFilterTest::testFilterNotAuthorized
Test code or tested code did not (only) close its own output buffers

phpvfscomposer:///home/runner/work/shield/shield/vendor/phpunit/phpunit/phpunit:106

5) Tests\Authentication\Filters\GroupFilterTest::testFilterIncorrectGroupNoPrevious
Test code or tested code did not (only) close its own output buffers

phpvfscomposer:///home/runner/work/shield/shield/vendor/phpunit/phpunit/phpunit:106

6) Tests\Authentication\Filters\GroupFilterTest::testFilterNotAuthorized
Test code or tested code did not (only) close its own output buffers

phpvfscomposer:///home/runner/work/shield/shield/vendor/phpunit/phpunit/phpunit:106

7) Tests\Authentication\Filters\TokenFilterTest::testBlocksInactiveUsers
Test code or tested code did not (only) close its own output buffers

phpvfscomposer:///home/runner/work/shield/shield/vendor/phpunit/phpunit/phpunit:106

8) Tests\Authentication\Filters\TokenFilterTest::testFiltersProtectsWithScopes
Test code or tested code did not (only) close its own output buffers

phpvfscomposer:///home/runner/work/shield/shield/vendor/phpunit/phpunit/phpunit:106

9) Tests\Authentication\Filters\TokenFilterTest::testFilterNotAuthorized
Test code or tested code did not (only) close its own output buffers

phpvfscomposer:///home/runner/work/shield/shield/vendor/phpunit/phpunit/phpunit:106

10) Tests\Authentication\ForcePasswordResetTest::testRequiresPasswordResetRedirect
Test code or tested code did not (only) close its own output buffers

phpvfscomposer:///home/runner/work/shield/shield/vendor/phpunit/phpunit/phpunit:106

11) Tests\Authentication\Filters\JWTFilterTest::testFilterNotAuthorized
Test code or tested code did not (only) close its own output buffers

phpvfscomposer:///home/runner/work/shield/shield/vendor/phpunit/phpunit/phpunit:106

12) Tests\Authentication\Filters\PermissionFilterTest::testFilterIncorrectGroupNoPrevious
Test code or tested code did not (only) close its own output buffers

phpvfscomposer:///home/runner/work/shield/shield/vendor/phpunit/phpunit/phpunit:106

13) Tests\Authentication\Filters\PermissionFilterTest::testFilterNotAuthorized
Test code or tested code did not (only) close its own output buffers

phpvfscomposer:///home/runner/work/shield/shield/vendor/phpunit/phpunit/phpunit:106

14) Tests\Controllers\ActionsTest::testEmail2FACannotBeBypassed
Test code or tested code did not (only) close its own output buffers

phpvfscomposer:///home/runner/work/shield/shield/vendor/phpunit/phpunit/phpunit:106

15) Tests\Controllers\ActionsTest::testEmailActivateCannotBeBypassed
Test code or tested code did not (only) close its own output buffers

phpvfscomposer:///home/runner/work/shield/shield/vendor/phpunit/phpunit/phpunit:106

OK, but incomplete, skipped, or risky tests!
Tests: 289, Assertions: [74](https://github.com/codeigniter4/shield/actions/runs/5984011237/job/16234766918?pr=801#step:11:75)5, Risky: 15.

@datamweb datamweb added the enhancement New feature or request label Aug 26, 2023
datamweb
datamweb reviewed Aug 26, 2023
View reviewed changes
Copy link
Collaborator

@datamweb datamweb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, I am against the views becoming crowded. The reason for this is that I believe that every website should have its own customized views, and it is usually easier to copy paste and edit simple files.

However, I understand what your goal is with this PR. So except for the one I mentioned, everything looks good.

Thanks for the screenshot.

@sammyskills
Copy link
Contributor Author

In general, I am against the views becoming crowded. The reason for this is that I believe that every website should have its own customized views, and it is usually easier to copy paste and edit simple files.

I understand your concern but it may surprise you to know that some of the websites that use shield, use the auth views the same way it is.

Currently, the views are built using bootstrap framework and a lot of websites use the same framework. So if there will be changes, it will be minimal.

Do you remember someone suggested that dark mode feature should be added to the views #772 ?

@datamweb datamweb closed this Aug 26, 2023
@datamweb datamweb reopened this Aug 26, 2023
datamweb
datamweb reviewed Aug 26, 2023
View reviewed changes
src/Language/fa/Auth.php Outdated Show resolved Hide resolved
@datamweb
Copy link
Collaborator

but it may surprise you to know that some of the websites that use shield, use the auth views the same way it is.

Yes, I know, probably we could have done much better in terms of education.
Worse than this, I see that many websites provide services to users in development mode.

Do you remember someone suggested that dark mode feature should be added to the views #772 ?

I was silent there too. Probably, if someone else had sent this PR, I would not have given these explanations. Anyway, there should be a difference between the answer to the user who has already been with the Shield and the new user who wants to be added recently.

datamweb
datamweb approved these changes Aug 26, 2023
View reviewed changes
Copy link
Collaborator

@datamweb datamweb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sammyskills thank you!

@sammyskills
Copy link
Contributor Author

sammyskills commented Aug 26, 2023 •
edited
Loading

I was silent there too. Probably, if someone else had sent this PR, I would not have given these explanations. Anyway, there should be a difference between the answer to the user who has already been with the Shield and the new user who wants to be added recently.

I get your point @datamweb.

There are some suggestions that are totally out of the scope of shield, but, I also know that there are some that will serve as a foundation or example to any tweak that will be done by developers.

As developers, I know it is sometimes hard to think about user experience, but, believe me, it is very vital.

@kenjis
Copy link
Member

kenjis commented Aug 27, 2023

Thank you. This is good. Because it is better to add label for input.
PhpStorm always warns these.
Screenshot 2023-08-27 11 48 38

kenjis
kenjis reviewed Aug 27, 2023
View reviewed changes
src/Language/ja/Auth.php Outdated Show resolved Hide resolved
@kenjis
Copy link
Member

kenjis commented Aug 29, 2023

Looks good!
Screenshot 2023-08-29 17 40 13

@kenjis kenjis merged commit b6d327d into codeigniter4:develop Aug 29, 2023
17 of 29 checks passed
@sammyskills sammyskills deleted the feat-improve-ux branch August 29, 2023 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@datamweb datamweb datamweb approved these changes

@kenjis kenjis kenjis approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sammyskills @datamweb @kenjis